Skip to content

feat: Add client side handlers to handle pagination + fix: Support pagination in tools change notification handler #306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

feat: Add client side handlers to handle pagination + fix: Support pagination in tools change notification handler #306

wants to merge 6 commits into from
+380 −75

Conversation

pantanurag555
Copy link
Contributor

@pantanurag555 pantanurag555 commented Jun 11, 2025 •
edited
Loading

Motivation and Context

Tools change notification handler is meant to send the updated list of tools to all consumers (clients) who have signed up for tools change notifications. In the current implementation in java-sdk, this would only return the first page of tools to the consumer. This PR:

  1. Introduces a fix to ensure that the tools change notification handler deals with the pagination for list tools and returns a list of all tools across pages back to the consumer.
  2. Introduces list handlers for the MCP client that can be used to obtain a list of all tools, resources, prompts and resource templates respectively without making multiple list calls to deal with pagination.

How Has This Been Tested?

Wrote a unit test that broke down list tools result across pages. Tested with the code before and after the change introduced in the PR. The old code (without the PR change) fails to return the complete list of tools. The new code (with the change) successfully returns all the tools across all pages.

Breaking Changes

None

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@LucaButBoring LucaButBoring mentioned this pull request Jun 16, 2025
Closed
LucaButBoring
LucaButBoring suggested changes Jun 17, 2025
View reviewed changes
pantanurag555 and others added 2 commits June 17, 2025 18:39
@pantanurag555
Merge branch 'modelcontextprotocol:main' into fix-notification-handler
62d1552
@pantanurag555
Add client side list handlers to handle pagination
86a9579 
Add client side list handlers to handle pagination for prompts,
resources and resource templates.
@pantanurag555 pantanurag555 changed the title fix: Support list tools pagination in tools change notification handler Add client side handlers to handle list pagination + fix: Support pagination in tools change notification handler Jun 19, 2025
@pantanurag555 pantanurag555 changed the title Add client side handlers to handle list pagination + fix: Support pagination in tools change notification handler fest: Add client side handlers to handle list pagination + fix: Support pagination in tools change notification handler Jun 19, 2025
@pantanurag555 pantanurag555 changed the title fest: Add client side handlers to handle list pagination + fix: Support pagination in tools change notification handler feat: Add client side handlers to handle pagination + fix: Support pagination in tools change notification handler Jun 19, 2025
@tzolov tzolov self-assigned this Jun 23, 2025
@tzolov tzolov added this to the 0.11.0 milestone Jun 23, 2025
@tzolov
Copy link
Contributor

tzolov commented Jun 23, 2025

@pantanurag555, thanks for addressing this.

We already have proper pagination APIs for all server resources:

One can use listXyz(null) for the first page, then pass the returned cursor for subsequent pages. For convenience, we could add public final static String FIRST_PAGE = null; to the McpSchema.

The non-paginated methods (listTools(), listResources(), etc.) should return all items but aren't implemented correctly.
IMO, we should move the implementations: listAllPrompts() → listPrompts(), listAllTools() → listTools(), etc., and remove the listAllXyz() methods.

@pantanurag555
Copy link
Contributor Author

@pantanurag555, thanks for addressing this.

We already have proper pagination APIs for all server resources:

* [McpAsyncClient#listTools(cursor)](https://github.com/modelcontextprotocol/java-sdk/blob/0fe76ba993799e8820f13311347e27692b51ae3b/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java#L671)

* [McpAsyncClient#listResources(cursor)](https://github.com/modelcontextprotocol/java-sdk/blob/0fe76ba993799e8820f13311347e27692b51ae3b/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java#L729)

* [McpAsyncClient#listResourceTemplates(cursor)](https://github.com/modelcontextprotocol/java-sdk/blob/0fe76ba993799e8820f13311347e27692b51ae3b/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java#L790)

* [McpAsyncClient#listPrompts(cursor)](https://github.com/modelcontextprotocol/java-sdk/blob/0fe76ba993799e8820f13311347e27692b51ae3b/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java#L866)

One can use listXyz(null) for the first page, then pass the returned cursor for subsequent pages. For convenience, we could add public final static String FIRST_PAGE = null; to the McpSchema.

The non-paginated methods (listTools(), listResources(), etc.) should return all items but aren't implemented correctly. IMO, we should move the implementations: listAllPrompts() → listPrompts(), listAllTools() → listTools(), etc., and remove the listAllXyz() methods.

That makes sense. Let me move around the implementations and make the relevant changes.

@pantanurag555 pantanurag555 force-pushed the fix-notification-handler branch from 28a678b to 86a9579 Compare June 23, 2025 19:55
tzolov pushed a commit that referenced this pull request Jun 24, 2025
@pantanurag555 @tzolov
feat: Improve pagination support on client side (#306)
d4a1bfa 
This change enhances the client API by providing both paginated access for fine-grained
control and convenience methods for retrieving complete result sets automatically.

- Implement "list all" methods that automatically handle pagination using expand/reduce operations
- Replace null parameters with McpSchema.FIRST_PAGE constant for better readability
- Add tests for both paginated and "list all" functionality
- Update method javadocs to clarify pagination behavior
- Maintain backward compatibility

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
@tzolov
Copy link
Contributor

tzolov commented Jun 24, 2025

Thanks @pantanurag555 !

Rebased, squashed, cleaned and merged at d4a1bfa

@tzolov tzolov closed this Jun 24, 2025
@pantanurag555 pantanurag555 deleted the fix-notification-handler branch June 24, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LucaButBoring LucaButBoring LucaButBoring requested changes

Assignees

@tzolov tzolov

Labels
None yet
Projects
None yet
Milestone
0.11.0
Development

Successfully merging this pull request may close these issues.
3 participants
@pantanurag555 @tzolov @LucaButBoring